Skip to content

Add: skills doctor subcommand + post-install routing repair (closing — superseded by branch rebase)#141

Closed
hjick wants to merge 2 commits into
pbakaus:mainfrom
hjick:fix/skills-doctor
Closed

Add: skills doctor subcommand + post-install routing repair (closing — superseded by branch rebase)#141
hjick wants to merge 2 commits into
pbakaus:mainfrom
hjick:fix/skills-doctor

Conversation

@hjick
Copy link
Copy Markdown

@hjick hjick commented May 7, 2026

Problem

Users who install impeccable globally via the recommended `npx skills add pbakaus/impeccable` flow can hit a routing bug in the upstream `skills` package: the skill lands in `/.agents/skills/impeccable/` but not in `/.claude/skills/impeccable/`. Claude Code only reads from `~/.claude/skills/`, so the skill never activates and users see no `/impeccable` commands. I personally hit this — silent failure for a few days until I traced it to the install location.

This is a known upstream issue: vercel-labs/skills#851 (and #693, #1045, #744). I have an upstream PR open at vercel-labs/skills#1089 that fixes the root cause (the interactive prompt did not pre-check Claude Code, so users hitting Enter installed only `.agents/skills` universal agents). Until that lands and ships, this PR adds a defensive layer here.

Changes

`npx impeccable skills doctor [--fix]`

A new diagnostic subcommand. Scans `//skills/` for impeccable installs and detects the routing mismatch: `/.agents/skills/` exists but `~/.claude/skills/` is missing. Reports the issue and (with `--fix`) creates the symlink. Idempotent. Honors prefixed names like `i-impeccable`. Treats non-TTY `--fix` as explicit consent so it composes with CI / scripts.

Auto-detect after install

`impeccable skills install` now calls the same diagnostic after the upstream `npx skills add` returns. If a misrouted install is detected, the user is prompted (or auto-repaired in `-y` mode). Wrapped in `try/catch` so the diagnostic step can never break install.

Docs

README gains a Troubleshooting subsection under Installation explaining the symptom, the upstream issue link, and the doctor command.

Tests

`tests/skills-doctor.test.js` — 6 cases mirroring the existing `tests/skills-cli.test.js` pattern (`bun:test`, real temp HOME directories):

  • clean state (no installs)
  • detects missing `/.claude/skills/impeccable` when `/.agents/skills/impeccable` exists
  • clean when both exist
  • `--fix` creates the symlink
  • handles prefixed `i-impeccable`
  • idempotent (running `--fix` twice does not error)

Verification

```sh
bun test tests/skills-doctor.test.js # 6 pass
bun run test # 186 pass (Node) — pre-existing failures in tests/skills-cli.test.js are unrelated to this change and reproduce on main
bun run build # validateProse + counts pass
```

Manual repro:

```sh
mkdir -p /tmp/hometest/.agents/skills/impeccable
HOME=/tmp/hometest node cli/bin/cli.js skills doctor --fix
test -L /tmp/hometest/.claude/skills/impeccable && echo OK
```

Notes


Note

Medium Risk
Medium risk because it adds new CLI behavior that scans the user’s home directory and can create symlinks during install/doctor flows; mistakes could affect local filesystem state.

Overview
Adds impeccable skills doctor [--fix] to diagnose the upstream npx skills routing bug where global installs land under ~/.agents/skills/ and Claude Code expects ~/.claude/skills/, with optional auto-repair via symlink creation; skills install now runs the same check after installing.

Updates CLI wiring and docs to match the repo’s new directory layout (notably skill/, cli/engine/, and site/), including adjusting live-mode server script lookup paths, .gitignore entries, and removing the legacy content/site/ editorial/tutorial markdown tree.

Reviewed by Cursor Bugbot for commit 02a135e. Bugbot is set up for automated code reviews on this repo. Configure here.

pbakaus and others added 2 commits May 7, 2026 17:48
* refactor(content): merge content/site/ into site/content/

Phase 1 step 1 of the directory restructure. The dual content tree was
called out in CLAUDE.md as cleanup; both trees were already in sync
except for anti-patterns-catalog.js, which moves to site/data/.

- Delete content/site/skills/ and content/site/tutorials/ (duplicates of
  site/content/, which is what Astro's content collection actually reads).
- Move content/site/anti-patterns-catalog.js -> site/data/.
- Update scripts/lib/sub-pages-data.js and scripts/build.js to read from
  site/content/ and site/data/.
- Drop content/site/ from validateProse target list (site/content was
  already there).
- Rewrite the "Two content trees" section in CLAUDE.md as a single-tree
  pointer; update stale dev-server text mentioning the deleted
  server/index.js.

Tests: 186/186 pass. Skills build: clean.

* refactor(skill): rename source/skills/impeccable/ -> skill/

Phase 1 step 2 of the directory restructure. The path was redundantly
nested ("source/" wrapper plus "skills/impeccable/" — singular content
hidden behind the plural). Collapses to flat skill/SKILL.md +
skill/reference/ + skill/scripts/.

- Move source/skills/impeccable/ -> skill/.
- Rewrite scripts/lib/utils.js readSourceFiles(): drop the multi-skill
  iteration (CLAUDE.md commits to a single user-invocable skill); read
  skill/SKILL.md directly.
- Update scripts/build.js, scripts/generate-og-image.js, and the
  sub-pages data layer to point at skill/.
- Update tests/lib/utils.test.js: drop the "multi-skill" and "dir-name
  fallback" cases, update single-skill paths to skill/.
- Update tests/build.test.js similarly: drop "multiple skills"
  integration test, update paths.
- Update non-glob path joins in tests/framework-fixtures.test.mjs,
  tests/live-e2e/session.mjs, tests/live-e2e/agents/llm-agent.mjs,
  tools/live-loop.mjs.
- Update prose/text references in CLAUDE.md, AGENTS.md, DEVELOP.md,
  README.md, scripts/lib/sub-pages-data.js, bin/commands/skills.mjs,
  site/data/anti-patterns-catalog.js, site/pages/docs/[...slug].astro,
  docs/adr-live-variant-mode.md, docs/plans/.

Eval framework note: the separate impeccable-evals repo reads
../impeccable/source/skills/impeccable/ and needs a coordinated
rename to ../impeccable/skill/.

Tests: 186/186 pass. Skills build: clean.

* refactor: rename docs/ -> notes/

Phase 1 step 3 of the directory restructure. The internal docs/ dir
(ADRs and plans) clashed with the site's /docs route. Renaming it
"notes/" makes the difference unambiguous: notes/ is project-internal
process, /docs is the user-facing route under site/pages/docs/.

No code references the dir; the rename is a clean git mv.

* refactor(site): move public/ under site/public/

Phase 2 step 4 of the directory restructure. Public assets and the
Astro publicDir now live alongside the rest of the site, so site/
is fully self-contained for static content.

- git mv public site/public.
- astro.config.mjs: add publicDir: './site/public'. Astro defaults to
  ./public at the project root, so the override is required.
- scripts/build.js: write generated _data, _headers, _redirects,
  _routes.json, and js/detect-antipatterns-browser.js into
  site/public/. Also delete the dead _REMOVED() Bun static-site
  builder (replaced by Astro at pbakaus#130; the placeholder no longer earns
  its keep).
- scripts/build.js validateProse: replace the stale public/index.html
  reference (deleted at the Astro migration) with site/pages/index.astro
  in the count-validation file list, restoring homepage drift detection.
- scripts/generate-og-image.js: write OG image into site/public/.
- scripts/screenshot-antipatterns.js: read examples from + write
  screenshots to site/public/antipattern-{examples,images}/.
- scripts/lib/sub-pages-data.js: load command demos from
  site/public/js/demos/commands.
- .gitignore: rename the public/* generator-output entries to
  site/public/*.
- CLAUDE.md: refresh CSS/data-file paths (still pointing at the old
  pre-Astro public/css/ + public/js/ tree), point the changelog and
  command-add checklists at site/pages/index.astro and
  site/scripts/data.js + site/scripts/components/framework-viz.js.

Cloudflare Pages note: functions/ stays at the repo root because
CF Pages auto-discovers it there with no configuration knob to
relocate. Moving it under site/ would either break deployment or
require a build-time copy step that adds more complexity than the
cleanup is worth.

Tests: 186/186 pass. Skills + site build clean. _headers,
_redirects, _routes.json, _data/ all land in build/ correctly.

* refactor(cli): consolidate bin/ + src/ + lib/ under cli/

Phase 2 step 5 of the directory restructure. The CLI surface was split
across three top-level dirs whose names were easy to mistake for each
other (especially src/ vs source/ pre-step-2). Consolidates under cli/.

- git mv bin -> cli/bin (CLI entry + skills sub-command)
- git mv src -> cli/engine (detect-antipatterns engine + browser variant)
- git mv lib -> cli/lib (download-providers helper)

Update package.json:
- bin.impeccable: cli/bin/cli.js
- main + exports: cli/engine/detect-antipatterns.mjs and the
  ./browser variant
- files: ["cli/", "LICENSE"]

Update internal references:
- cli/bin/cli.js: dynamic import points at ../engine/, package.json
  read goes one level deeper (../../package.json).
- functions/api/download/[type]/[provider]/[id].js + bundle/[provider].js:
  cli/lib/download-providers.js path.
- scripts/build.js, scripts/build-browser-detector.js,
  scripts/build-extension.js: cli/engine path constants.
- scripts/lib/sub-pages-data.js, scripts/lib/utils.js, skill/scripts/
  live-server.mjs: comment refs.
- tests/detect-antipatterns{,-browser,-fixtures}.test.{js,mjs},
  tests/windows-path-fix.test.js: import + read paths.
- AGENTS.md, CLAUDE.md: doc paths.

Verified:
- npx node cli/bin/cli.js --version, --help, detect --help all work.
- bun run build, bun run build:browser, bun run build:extension all
  clean. Browser detector lands at cli/engine/detect-antipatterns-browser.js;
  extension/detector/detect.js still emits to the same location.
- bun run test: 186/186 pass.

* fix: update browser-detector paths missed in cli/ rename

Bugbot caught two runtime path leaks where the comment got renamed
to cli/engine/ but the actual code still used the old src/ segment.

- skill/scripts/live-server.mjs: detectPaths array now joins cli, engine,
  detect-antipatterns-browser.js for both the repo-relative lookup
  (4 dirs up from .claude/skills/impeccable/scripts/ to repo root) and
  the npm node_modules fallback. Without this fix, the detection
  overlay would silently not load during live-server sessions.

- scripts/build.js: the post-build copy of the browser detector into
  site/public/js/ was reading from src/. The if (fs.existsSync(...))
  guard meant the copy was silently skipping, so antipattern-examples
  pages would 404 on /js/detect-antipatterns-browser.js once the site
  was deployed.

Tests: 186/186 pass. Build clean. site/public/js/detect-antipatterns-browser.js
re-emits as expected.

* fix: cleanup-deprecated import path missed an extra .. in cli/ rename

Bugbot caught three call sites in cli/bin/commands/skills.mjs that
import '../../skill/scripts/cleanup-deprecated.mjs'. Pre-rename, that
was correct from bin/commands/ (one parent to bin/, one to repo root).
After moving the file from bin/commands/ to cli/bin/commands/, the
path is one directory deeper, so it needs three .. segments to reach
the repo root. Without the fix, every cleanup invocation throws on
import and gets swallowed by the surrounding try/catch — silent skip.

cli/bin/cli.js's package.json read already uses '../../package.json'
(the same depth pattern), confirming three levels is correct.

Verified: dynamic import resolves and exports the expected functions.

* chore: sweep stale path/file references missed in the restructure

Same root cause as the two bugbot finds: some references in moved or
related files weren't tracked because they didn't match a simple
sed pattern. Caught the rest by walking each moved dir's depth and
each Astro-migration deletion.

Stale path references (post-Astro migration, missed earlier):
- CLAUDE.md: legacy URL redirects "live in server/index.js" -> point
  at the actual sources (scripts/build.js generateCFConfig +
  site/public/_redirects).
- AGENTS.md: counts.js path (public/ -> site/public/), changelog file
  (public/index.html -> site/pages/index.astro), screenshots note
  (public/ -> site/), source-of-truth dirs (source/, src/ -> skill/,
  cli/).
- tests/detect-antipatterns-browser.test.mjs: comment about routes
  "in server/index.js".
- skill/reference/live.md: workflow.css example for "this repo" was
  pre-Astro (public/css/) -> site/styles/. (User-project Vite/Next
  example unchanged.)

Stale path that pointed at moved files:
- tests/skills-cli.test.js: CLI path was '..', 'bin', 'cli.js'; now
  '..', 'cli', 'bin', 'cli.js'. Test isn't wired into bun run test
  but it would have failed if invoked.

Dead files (orphaned by Astro migration, never cleaned up):
- tests/server/download-validation.test.js: imported from
  ../../server/lib/{validation,api-handlers}.js which were deleted in
  b8f09c8. Test was a silent failure waiting to happen.
- scripts/lib/render-markdown.js: 156-line module with zero consumers
  (the only caller, scripts/lib/render-page.js, was deleted in the
  Astro cleanup).
- scripts/build.js: dead commented-out generateSubPages import.

Tests: 186/186 pass. Build clean.

* fix(build): remove invalid Corepack packageManager spec

Cloudflare Pages rejects the build with `Unsupported package manager
specification (bun@1.3.11)`. The packageManager field follows
Corepack's syntax which only validates npm/pnpm/yarn — `bun@X.Y.Z`
parses as a malformed Corepack directive even though Bun itself
treats it as a hint.

Pre-existing on main since d874af0 (CF Pages deploy on main also
failing); just surfaces here because the PR triggers a fresh deploy.

CF Pages auto-detects Bun anyway (the build log confirms:
"Detected the following tools from environment: bun@1.3.11,
pnpm@10.11.1, nodejs@22.16.0"). Removing the field unblocks the
deploy without changing local dev behavior.

---------

Co-authored-by: Paul Bakaus <paulbakaus@pauls-mbp-3.lan>
상류 `npx skills` (vercel-labs/skills#851) 가 Claude Code 글로벌 설치를
~/.agents/skills/ 로만 보내고 ~/.claude/skills/ 에 symlink 를 만들지 않는
경우, Claude Code 가 스킬을 못 보는 문제가 있다. 상류 PR 이 머지되어
배포되기 전까지 사용자 보호용 방어 코드를 추가한다.

- `npx impeccable skills doctor [--fix]` 추가. 모든 PROVIDER_DIRS 의
  글로벌 설치 위치를 스캔해 ~/.agents/skills/<name> 만 있고
  ~/.claude/skills/<name> 이 없는 경우를 감지하고, --fix 로 symlink
  를 만든다. 비-TTY 환경에서는 --fix 가 명시적 동의로 간주됨.
- `impeccable skills install` 마지막에 동일 진단을 수행하고, 문제가
  있으면 사용자에게 자동 복구를 제안한다 (-y 모드는 무자각 자동).
- 진단/복구 로직은 try/catch 로 감싸 install 본 흐름을 절대 깨지
  않게 한다.
- 파일별: cli/bin/commands/skills.mjs 핵심 로직, cli/bin/cli.js
  헬프 갱신, README.md 트러블슈팅 섹션 추가, tests/skills-doctor.test.js
  6 케이스 회귀 테스트.
@hjick hjick requested a review from pbakaus as a code owner May 7, 2026 09:01
@hjick hjick changed the title Add: skills doctor subcommand + post-install routing repair Add: skills doctor subcommand + post-install routing repair (closing — superseded by branch rebase) May 7, 2026
@hjick
Copy link
Copy Markdown
Author

hjick commented May 7, 2026

Superseded by #142 (rebased onto current main).

@hjick hjick closed this May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants